Skip to content

feat(uffd,fc): wire balloon free-page-reporting#2541

Open
ValentaTomas wants to merge 13 commits intomainfrom
feat/uffd-fc-free-page-reporting-integration
Open

feat(uffd,fc): wire balloon free-page-reporting#2541
ValentaTomas wants to merge 13 commits intomainfrom
feat/uffd-fc-free-page-reporting-integration

Conversation

@ValentaTomas
Copy link
Copy Markdown
Member

@ValentaTomas ValentaTomas commented May 3, 2026

Wires Firecracker balloon free-page-reporting through the template build into pkg/sandbox/fc. In the orchestrator path FPR turns on only when fcInfo.HasFreePageReporting() (FC v1.14+) AND the free-page-reporting LaunchDarkly flag are both true; cmd/create-build defaults to the FC-version-derived value and accepts an explicit --free-page-reporting override.

Depends on #2545#2520 — without the REMOVE-event handling from #2520, FC's madvise(MADV_DONTNEED) on balloon deflate would race in-flight pagefault workers.

@cursor
Copy link
Copy Markdown

cursor Bot commented May 3, 2026

PR Summary

Medium Risk
Touches Firecracker boot/configuration and snapshot dirty/empty page accounting; mistakes could cause incorrect diffs or runtime memory behavior. Gated by FC version check and a feature flag, reducing blast radius but still affects core sandbox lifecycle when enabled.

Overview
Adds Firecracker balloon device installation to enable free-page-reporting, wiring a new FreePageReporting option from template creation/build config through sandbox creation into fc.Process.Create, gated by FC >= 1.14 and a new free-page-reporting feature flag.

Changes snapshot diff collection when using UFFD to combine UFFD “removed/zero” tracking with Firecracker DirtyMemory, including new ExportPageStates settling to avoid races; incorrect ordering or bitmap merging could misclassify pages as empty vs dirty (data loss in diffs) if assumptions don’t hold under load.

Reviewed by Cursor Bugbot for commit 7df828b. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread packages/shared/pkg/fcversion/sandbox_features.go
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from 0a2570e to 99eec35 Compare May 3, 2026 02:21
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from 0fd4caf to d30e0ef Compare May 3, 2026 04:52
@ValentaTomas ValentaTomas changed the title feat(uffd): wire free-page-reporting through template build to FC balloon feat(uffd,fc): wire balloon free-page-reporting (auto for FC v1.14+) May 3, 2026
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch 3 times, most recently from bf23242 to 6d4b734 Compare May 3, 2026 05:28
@ValentaTomas ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from 68037d2 to ceec7d6 Compare May 3, 2026 07:10
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from d38614d to c777e5c Compare May 3, 2026 07:18
@ValentaTomas ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from ceec7d6 to 9ce0941 Compare May 3, 2026 07:36
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from c777e5c to 6faaefa Compare May 3, 2026 07:39
@ValentaTomas ValentaTomas marked this pull request as ready for review May 3, 2026 08:04
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6faaefab87

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/orchestrator/pkg/sandbox/uffd/uffd.go Outdated
Comment thread packages/orchestrator/pkg/sandbox/uffd/uffd.go Outdated
@ValentaTomas ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from 9ce0941 to 5896b7b Compare May 3, 2026 08:43
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from 6faaefa to 4bb8be8 Compare May 3, 2026 08:43
@ValentaTomas ValentaTomas changed the title feat(uffd,fc): wire balloon free-page-reporting (auto for FC v1.14+) feat(uffd,fc): wire balloon free-page-reporting May 3, 2026
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch 2 times, most recently from 07ab37d to 4951164 Compare May 3, 2026 09:39
@ValentaTomas ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from 345f7e9 to db978d4 Compare May 3, 2026 10:22
Read the Removed bitmap from PageTracker and emit it as DiffMetadata.Empty
so REMOVE'd pages become uuid.Nil mappings in the snapshot header (read as
zero on resume). Defensively AndNot the empty set out of dirty: settle drains
make these disjoint in practice (Removed pages have no PTE, WP-async only
sees present pages with WP cleared), but if the invariant ever breaks the
guest's last intent for a Removed page is "free, read zero on restore" — so
empty must win, not stale dirty content.
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from 920e8ec to 7f22709 Compare May 5, 2026 08:19
Comment thread packages/orchestrator/pkg/template/server/create_template.go
…e comments

Receiver derives freePageReporting from FC version + LD flag and never
reads cfg.GetFreePageReporting(), so the proto field was dead. Removing
it also kills the noisy generated pb.go diff.
Comment thread packages/orchestrator/pkg/sandbox/uffd/uffd.go Outdated
A REMOVE'd page that gets a read fault before snapshot installs
zero+WP+wake. The unconditional Zero->Dirty promotion stripped it from
the empty bitmap while WP-async dirty scan still excludes it (WP set,
no write yet), so the page fell back to the parent layer's stale
content. Keep the tracker entry as Zero for that case and let WP-async
surface any later write; flip the AndNot so dirty wins over empty for
written pages.
Comment thread packages/orchestrator/pkg/sandbox/uffd/uffd.go
Comment thread packages/orchestrator/pkg/template/server/create_template.go
Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go
- DiffMetadata: settle UFFD workers before sampling FC WP-async pagemap so
  a Zero->Write install in flight can't escape both Dirty and Empty bitmaps.
- TemplateCreate: gate freePageReporting on !hugePages. Firecracker rejects
  balloon device + hugepages combinations, and the v1.14 rollout would
  otherwise auto-enable both and abort VM creation.
@cla-bot cla-bot Bot added the cla-signed label May 5, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a092da952a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/orchestrator/pkg/template/server/create_template.go Outdated
Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/userfaultfd.go
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.

Once credits are available, push a new commit or reopen this pull request to trigger a review.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ff748eafd3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/orchestrator/pkg/template/server/create_template.go Outdated
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 6, 2026

Comment thread packages/orchestrator/pkg/template/server/create_template.go Outdated
Remove the !hugePages guard — hugepages and balloon FPR are not
mutually exclusive in Firecracker.
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization has reached its monthly code review spending cap.

An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.

Once the cap resets or is raised, push a new commit or reopen this pull request to trigger a review.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.

Reviewed by Cursor Bugbot for commit 14f995f. Configure here.

Comment thread packages/orchestrator/pkg/template/server/create_template.go
@ValentaTomas ValentaTomas requested a review from bchalios May 7, 2026 06:25
Generic name and parameterized FreePageReporting so individual balloon
features (FPR today, FPH next) can be opted in independently from the
caller without renaming the helper again.
@ValentaTomas ValentaTomas removed the request for review from dobrac May 8, 2026 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants